Skip to content

[TECH] arrêter d'utiliser les champs userId / snappedAt (Pix-17286) #11965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

lionelB
Copy link
Member

@lionelB lionelB commented Apr 3, 2025

🌸 Problème

Dans le cadre de la mise en conformité des données, On souhaite remplacer l'utilisation des champs userIdet snappedAt par campaignParticipationId.

🌳 Proposition

On arrête de peupler et d'utiliser les champs userIdet snappedAt

🐝 Remarques

🤧 Pour tester

  • se connecter avec admin-orga@example.net
  • participer à la campagne PROCOLMUL
  • partager ses résultats
    puis
  • lancer la commande
scalingo -a pix-api-review-pr11965 pgsql-console
  • lancer la requete
select id, "userId", "snappedAt", "campaignParticipationId" from "knowledge-element-snapshots" where "campaignParticipationId" = 133227
  • valider que les champs userId et snappedAt son vide :)

@lionelB lionelB self-assigned this Apr 3, 2025
@lionelB lionelB added the ⚠️ PR Inheritance This PR inherits a first-to-merge PR and will need a rebase label Apr 3, 2025
@lionelB lionelB force-pushed the pix-17286/stop-populate-userId-snappedAt branch from de19255 to fcaa381 Compare April 3, 2025 16:02
@lionelB lionelB removed the ⚠️ PR Inheritance This PR inherits a first-to-merge PR and will need a rebase label Apr 3, 2025
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@theotime2005 theotime2005 force-pushed the pix-17286/stop-populate-userId-snappedAt branch from c13c00c to 77db09e Compare April 4, 2025 12:42
@xav-car xav-car force-pushed the pix-17286/stop-populate-userId-snappedAt branch from 55c92c2 to 3f4bbd7 Compare April 4, 2025 13:40
@xav-car xav-car marked this pull request as ready for review April 4, 2025 13:41
@xav-car xav-car requested review from a team as code owners April 4, 2025 13:41
@xav-car xav-car force-pushed the pix-17286/stop-populate-userId-snappedAt branch from 3f4bbd7 to 4d7f77a Compare April 4, 2025 13:56
@xav-car xav-car added 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally and removed Development in progress labels Apr 4, 2025
snapshot,
campaignParticipationId,
} = {}) {
const dateMinusOneDay = new Date(snappedAt.getTime() - 1000 * 60 * 60 * 24 * 7);
userId = _.isUndefined(userId) ? buildUser().id : userId;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: on peut conserver le userId pour la construction des knowledgeElement

createdAt,
});

databaseBuilder.factory.buildKnowledgeElementSnapshot({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question : ce ke n'etait utilisé ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vu que dans le contexte général, on ne teste que le nom du fichier d'export, ça faisait sens de ne pas placer la création de ke ici (ils sont toujours construits dans les autres contextes).

'campaign-participations.sharedAt',
);
})
.select('campaign-participations.id', 'campaign-participations.userId', 'campaign-participations.sharedAt')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question Est ce qu'on a encore besoin de ce script (on pourrait le supprimer si il n'est plus utile)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je crois qu'on l'a laissé parce qu'il pourrait nous resservir

@alicegoarnisson alicegoarnisson added Func Review OK PO validated functionally the PR and removed 👀 Func Review Needed Need PO validation for this functionally labels Apr 8, 2025
@alicegoarnisson alicegoarnisson force-pushed the pix-17286/stop-populate-userId-snappedAt branch from 657bd57 to 8b5056f Compare April 8, 2025 08:44
Copy link
Contributor

@machestla machestla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tech review ok ✅ 🐕

if (!snapshot) {
const knowledgeElements = [];
knowledgeElements.push(buildKnowledgeElement({ userId, createdAt: dateMinusOneDay }));
knowledgeElements.push(buildKnowledgeElement({ userId, createdAt: dateMinusOneDay }));
userId = _.isUndefined(userId) ? buildUser().id : userId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought : avons nous besoin de lodash pour des questions de lisibilité ou est ce que l'écriture userId === undefined pourrait répondre au besoin ?

@@ -1,6 +1,6 @@
import _ from 'lodash';

import * as knowledgeElementSnapshotAPI from '../../../../../../src/prescription/campaign/application/api/knowledge-element-snapshots-api.js';
import * as knowledgeElementSnapshotAPI from '../../../../../../src/prescription/campaign/application/api/knowledge-element-snapshots-api.js';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: étonnant que cela ne titille pas le linter

lionelB and others added 9 commits April 8, 2025 09:09
…pation

Co-Authored-By: alicegoarnisson <187311468+alicegoarnisson@users.noreply.github.com>
Co-Authored-By: Xavier Carron <33637571+xav-car@users.noreply.github.com>
…ent-snapshots

Co-Authored-By: alicegoarnisson <187311468+alicegoarnisson@users.noreply.github.com>
Co-Authored-By: Xavier Carron <33637571+xav-car@users.noreply.github.com>
…pository

Co-Authored-By: alicegoarnisson <187311468+alicegoarnisson@users.noreply.github.com>
Co-Authored-By: Xavier Carron <33637571+xav-car@users.noreply.github.com>
Co-Authored-By: Alexandre-Monney <101280231+Alexandre-Monney@users.noreply.github.com>
Co-authored-by: Théotime Berthod <theotime.berthod@pix.fr>
…pationId

Co-authored-by: Théotime Berthod <theotime.berthod@pix.fr>
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-17286/stop-populate-userId-snappedAt branch from 8b5056f to 36b3db5 Compare April 8, 2025 09:09
@pix-service-auto-merge pix-service-auto-merge merged commit 3271a9f into dev Apr 8, 2025
9 of 11 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-17286/stop-populate-userId-snappedAt branch April 8, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants